-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #3448 Improve Type inference for IFTs #3449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't proceed with implicit search if result type cannot match - the search will likely by under-constrained, which means that an unbounded number of alternatives is tried. See strawman-contrib MapDecoratorTest.scala for an example where this happens.
This caused nested ambiguity errors with the next commit.
Propagate implicit ambiguity and divergence failures from implicit arguments to their callers. In particular, and ambiguity in an implicit argument means that the whole implicit search is ambiguous. Previously, the local ambiguity translated to an "implicit not found" on the next outer level, which meant that another implicit on that level could qualify and the ambiguity would be forgotten. The code is still a bit rough and could profit from some refactorings.
This is a temporary fix. iterator-from seems to cause non-deterministic ambiguity errors. Moving to pending until we have figured out what goes wrong.
Refactoring of implicits with the aim of clearer code and better error messages. Here's an example: -- Error: implicitSearch.scala:15:12 ------------------------------------------- 15 | sort(xs) // error (with a partially constructed implicit argument shown) | ^ |no implicit argument of type Test.Ord[scala.collection.immutable.List[scala.collection.immutable.List[T]]] was found for parameter o of method sort in object Test. |I found: | | Test.listOrd[T](Test.listOrd[T](/* missing */implicitly[Test.Ord[T]])) | |But no implicit values were found that match type Test.Ord[T].
Turns out it's not transitive when combined with the old scheme. So it should be one or the other. Conservatively, this commit reverts to the old scheme.
The aim is to have ultimately fewer comparisons. The downside is that some of the comparisons are now more expensive, because types have to be compared where they previously weren't.
- Merge `rankImplicits` and `condense` into one phase - Fix handling of ambiguities - the previous unconditional abort was wrong, because another alternative might be better than both ambiguous choices.
All tests pass, and it is unclear what this is trying to achieve.
We now treat a type as an implicit function type also if it is a type variable that has an IFT as upper bound in the current constraint. This means we now create implicit closures if the expected type is such a type.
smarter
approved these changes
Nov 14, 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending merge of #3421
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We now treat a type as an implicit function type also if it is
a type variable that has an IFT as upper bound in the current constraint.
This means we now create implicit closures if the expected type is
such a type.
Fixes #3448.
Based on #3421.